Don't give APITs names with macro expansion placeholder fragments in it#142393
Merged
bors merged 1 commit intorust-lang:masterfrom Jun 27, 2025
Merged
Don't give APITs names with macro expansion placeholder fragments in it#142393bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
Collaborator
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
rustbot
added
S-waiting-on-review
Collaborator
|
This PR changes a file inside |
compiler-errors
commented
Jun 12, 2025
| LL | let () = x; | ||
| | ^^ - this expression has type `impl Foo<bar!()>` | ||
| | | | ||
| | expected type parameter `impl Foo<bar!()>`, found `()` |
Contributor
Author
There was a problem hiding this comment.
For contrast, this used to render like impl Foo<!()>, where !() is the placeholder fragment.
davidtwco
approved these changes
Jun 12, 2025
Member
|
r=me if @petrochenkov is happy since he's also assigned |
petrochenkov
added
S-waiting-on-author
Contributor
Author
|
flipping state back b/c of a pending question @rustbot ready |
rustbot
added
S-waiting-on-review
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
|
#142690 should help with #142393 (comment) |
rustbot
added
S-blocked
Contributor
Author
Yeah, I don't think it matters which node id was being used as a key. I think that PR + using the ty's node id is sufficient. |
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
Contributor
|
@bors r+ |
Collaborator
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 26, 2025
…enkov Don't give APITs names with macro expansion placeholder fragments in it The `DefCollector` previously called `pprust::ty_to_string` to construct a name for APITs (arg-position impl traits). The `ast::Ty` that was being formatted however has already had its macro calls replaced with "placeholder fragments", which end up rendering like `!()` (or ICEing, in the case of rust-lang#140333, since it led to a placeholder struct field with no name). Instead, collect the name of the APIT *before* we visit its macros and replace them with placeholders in the macro expander. This makes the implementation a bit more involved, but AFAICT there's no better way to do this since we can't do a reverse mapping from placeholder fragment -> original macro call AST. Fixes rust-lang#140333
bors
added a commit
that referenced
this pull request
Jun 26, 2025
Rollup of 9 pull requests Successful merges: - #124595 (Suggest cloning `Arc` moved into closure) - #139594 (Simplify `ObligationCauseCode::IfExpression`) - #141311 (make `tidy-alphabetical` use a natural sort) - #141648 ([rustdoc] Do not emit redundant_explicit_links lint if the doc comment comes from expansion) - #142285 (tests: Do not run afoul of asm.validity.non-exhaustive in input-stats) - #142393 (Don't give APITs names with macro expansion placeholder fragments in it) - #142884 (StableMIR: Add method to retrieve body of coroutine) - #142981 (Make missing lifetime suggestion verbose) - #143030 (Fix suggestion spans inside macros for the `unused_must_use` lint) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
that referenced
this pull request
Jun 26, 2025
Rollup of 8 pull requests Successful merges: - #124595 (Suggest cloning `Arc` moved into closure) - #139594 (Simplify `ObligationCauseCode::IfExpression`) - #141311 (make `tidy-alphabetical` use a natural sort) - #141648 ([rustdoc] Do not emit redundant_explicit_links lint if the doc comment comes from expansion) - #142285 (tests: Do not run afoul of asm.validity.non-exhaustive in input-stats) - #142393 (Don't give APITs names with macro expansion placeholder fragments in it) - #142884 (StableMIR: Add method to retrieve body of coroutine) - #142981 (Make missing lifetime suggestion verbose) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
that referenced
this pull request
Jun 26, 2025
Rollup of 8 pull requests Successful merges: - #124595 (Suggest cloning `Arc` moved into closure) - #139594 (Simplify `ObligationCauseCode::IfExpression`) - #141311 (make `tidy-alphabetical` use a natural sort) - #141648 ([rustdoc] Do not emit redundant_explicit_links lint if the doc comment comes from expansion) - #142285 (tests: Do not run afoul of asm.validity.non-exhaustive in input-stats) - #142393 (Don't give APITs names with macro expansion placeholder fragments in it) - #142884 (StableMIR: Add method to retrieve body of coroutine) - #142981 (Make missing lifetime suggestion verbose) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Jun 27, 2025
Rollup merge of #142393 - compiler-errors:nofield, r=petrochenkov Don't give APITs names with macro expansion placeholder fragments in it The `DefCollector` previously called `pprust::ty_to_string` to construct a name for APITs (arg-position impl traits). The `ast::Ty` that was being formatted however has already had its macro calls replaced with "placeholder fragments", which end up rendering like `!()` (or ICEing, in the case of #140333, since it led to a placeholder struct field with no name). Instead, collect the name of the APIT *before* we visit its macros and replace them with placeholders in the macro expander. This makes the implementation a bit more involved, but AFAICT there's no better way to do this since we can't do a reverse mapping from placeholder fragment -> original macro call AST. Fixes #140333
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
DefCollectorpreviously calledpprust::ty_to_stringto construct a name for APITs (arg-position impl traits). Theast::Tythat was being formatted however has already had its macro calls replaced with "placeholder fragments", which end up rendering like!()(or ICEing, in the case of #140333, since it led to a placeholder struct field with no name).Instead, collect the name of the APIT before we visit its macros and replace them with placeholders in the macro expander. This makes the implementation a bit more involved, but AFAICT there's no better way to do this since we can't do a reverse mapping from placeholder fragment -> original macro call AST.
Fixes #140333